Skip to content

fix(alerts): declare the event types each resource can deliver, reject and stop offering the rest - #30587

Open
manerow wants to merge 2 commits into
mainfrom
fix/alert-supported-event-types
Open

fix(alerts): declare the event types each resource can deliver, reject and stop offering the rest#30587
manerow wants to merge 2 commits into
mainfrom
fix/alert-supported-event-types

Conversation

@manerow

@manerow manerow commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Fixes #30557
Fixes #29040

Both halves of the "silent no-op" complaint in #27889: the server declares what each resource can
deliver and rejects the rest, and the alert builder offers only what the server declares. They ship
together so the UI can never present a combination the API will refuse.

Problem

An alert can be saved with event types its resource can never produce, and it then delivers nothing
with no error and no empty state. Reproduced on main:

alert a27889gtsuggestion1785226866
  resource: glossaryTerm
  rule:     matchAnyEventType({'suggestionCreated','suggestionAccepted'})
  events:   total 0, pending 0, successful 0, failed 0

Two blind spots:

  • Nothing declares what a resource emits. GET /v1/events/subscriptions/notification/resources
    returned only name, supportedFilters, supportedActions, containerEntities, so the builder
    falls back to the whole 26-value EventType enum.
  • Save-time validation checks filter names, not values. AlertUtil.getFilterRule rejects a
    filter the resource does not support, but never inspects the arguments inside it.

What each resource can actually deliver

Derived from the emitters rather than from the enum:

Resource Event types
entity resources (glossaryTerm, topic, …) entityCreated, entityUpdated, entitySoftDeleted, entityDeleted, entityRestored plus threadCreated, threadUpdated, postCreated, postUpdated, because #28122 routes a conversation to the alert of the entity it is about
table, dashboard, pipeline, chart, mlmodel the above plus entityFieldsChanged
conversation the thread family only
task entity family plus taskResolved / taskClosed, still emitted by the legacy Recognizer path (#30559 retires them)
all everything above plus logicalTestCaseAdded and entityLineageAdded/Updated/Deleted; it short-circuits the resource gate, so it is the only resource that sees events whose entityType (lineage, testSuite) is not a notification resource

Never declared, because nothing can deliver them: entityNoChange, taskCreated, taskUpdated,
the five suggestion*, userLogin, userLogout.

Three of those deserve a note, since they are easy to assume otherwise:

  • entityFieldsChanged is not generic. Only usage reporting emits it (UsageRepository:121-125), for exactly five entities. QueryRepository also emits it, but ChangeEventHandler:57-59 drops every query and workflow event before insert.
  • userLogin / userLogout never reach change_event. AuditLogRepository.writeAuthEvent builds a ChangeEvent, but write() only inserts an AuditLogRecord. Verified on a running instance: 22 logins, all in audit_log_event, none in change_event.
  • entityNoChange is a sentinel that ChangeEventHandler:77 explicitly skips.

Changes (server)

  • filterResourceDescriptor.json gains supportedEventTypes, so the descriptor already served at /notification/resources now carries the declaration and the UI needs no new endpoint.
  • New ResourceEventTypes encodes the table above: two family constants, a usage-resource set, explicit entries for all, conversation and task, and the entity+thread default for everything else. EventSubscriptionResource.getNotificationsFilterDescriptors() attaches it where it already maps filter names onto rules.
  • AlertUtil.validateAndBuildFilteringConditions looks the descriptor up once and hands it to both gates: the existing filter-name check, and the new event-type value check that reads descriptor.getSupportedEventTypes() and throws BadRequestException naming the resource and the value, with the message next to resourceTypeNotFound in CatalogExceptionMessage.

The input descriptor (EventSubResourceDescriptor.json) is deliberately untouched. The declaration is
derived rather than declared, following ResourceRegistry, which answers the same question for
authorization: a common base plus per-resource specifics plus an all entry, computed in Java, served
to a builder UI at /v1/policies/resources and enforced by PolicyEvaluator. As there, the rules are
a producer with a single consumer and every reader goes through the descriptor, so AlertUtil never
calls ResourceEventTypes directly. Twenty-eight hand-written lists is also how the existing
descriptor drifted: it still carries the dead tagCategory and lacks its replacement classification.

Observability is unaffected: EntityObservabilityFilterDescriptor.json offers no filterByEventType.

Changes (UI)

AlertsUtil.tsx built the Event Type field from the whole EventType enum
(getSelectOptionsFromEnum(EventType)), regardless of the selected resource. It now uses the
resource's supportedEventTypes, falling back to the enum when a resource declares none, which
keeps a newer UI working against an older server and leaves observability alone (its descriptors
carry no filterByEventType).

supportedEventTypes is threaded exactly like the existing containerEntities: derived where the
descriptor is already looked up (AddNotificationPage, and AlertConfigDetails for view mode),
passed into ObservabilityFormFiltersItem, and forwarded through getConditionalField into
getFieldByArgumentType. One new pure helper, getSelectOptionsFromValues, sits beside
getSelectOptionsFromEnum and uses the same startCase label transform, which keeps the Playwright
selectors ([title="${startCase(eventType)}"]) and the view-mode labels working.

No new i18n strings.

An alert saved before this change that stores a now-unadvertised value still shows it as a
removable chip, labelled with the raw value, so it can be cleared. Verified in the browser rather
than assumed.

Behaviour change worth noting in release notes

Enforcement is strict for create and update alike, for filters that include an event type. An
alert upgraded from an older release that includes a now-unreachable value (suggestionCreated,
taskResolved, …) must have that value removed before its next save; the 400 names it. The value was
already dead, so nothing that used to fire stops firing. #29039's migration clears those values
wholesale.

An EXCLUDE filter is left alone: excluding an event type the resource cannot produce is a harmless
no-op, not an alert that can never fire, so it is not worth blocking a save over.

Tests

  • AlertsUtil.test.tsx: the field offers only the values it is given, and falls back to the whole enum when given none.
  • ResourceEventTypesTest: per-resource assertions, plus an anti-drift test asserting that the union of every declared list and the never-deliverable set is exactly EventType.values(), so adding an enum value without deciding where it belongs fails the build.
  • EventTypeValidationTest: the gate, including table accepting entityFieldsChanged while glossaryTerm rejects it, and an unknown string being rejected rather than throwing.
  • EventSubscriptionResourceIT: 400 on an unreachable value naming it, a reachable value still saving, and /notification/resources serving supportedEventTypes over the wire.

Verified on a local deploy

glossaryTerm  [entityCreated, entityUpdated, entitySoftDeleted, entityDeleted, entityRestored,
               threadCreated, threadUpdated, postCreated, postUpdated]
conversation  [threadCreated, threadUpdated, postCreated, postUpdated]

glossaryTerm + suggestionCreated    -> 400 "Resource glossaryTerm does not produce event type
                                            suggestionCreated, so an alert filtering on it can never fire"
glossaryTerm + entityFieldsChanged  -> 400
glossaryTerm + threadCreated,postCreated -> 201
table + entityFieldsChanged              -> 201

And in the alert builder, on a Glossary Term alert: searching the Event Type dropdown for
"sugg" or "fields" returns No data, while "thread" offers Thread Created and Thread Updated.
The pre-existing alert that stores suggestionCreated still renders it as a removable chip.

Greptile Summary

The PR prevents notification subscriptions from selecting or saving event types their chosen resource cannot produce.

  • Adds server-derived supported event-type declarations to notification resource descriptors.
  • Enforces those declarations when notification subscriptions are created or updated.
  • Restricts the alert builder’s event-type options while retaining compatibility with older servers.
  • Adds backend, integration, and UI tests for declaration, validation, and fallback behavior.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
openmetadata-service/src/main/java/org/openmetadata/service/events/subscription/ResourceEventTypes.java Defines the event families advertised for generic and special notification resources.
openmetadata-service/src/main/java/org/openmetadata/service/events/subscription/AlertUtil.java Validates included event-type filter values against the selected resource descriptor.
openmetadata-service/src/main/java/org/openmetadata/service/resources/events/subscription/EventSubscriptionResource.java Adds supported event types to resource descriptors returned by the notification API.
openmetadata-spec/src/main/resources/json/schema/events/filterResourceDescriptor.json Extends the descriptor contract with the optional supportedEventTypes field.
openmetadata-ui/src/main/resources/ui/src/utils/Alerts/AlertsUtil.tsx Builds event-type choices from the server declaration and falls back to the complete enum when absent.
openmetadata-ui/src/main/resources/ui/src/pages/AddNotificationPage/AddNotificationPage.tsx Resolves and passes the selected resource’s supported event types into the alert form.
openmetadata-ui/src/main/resources/ui/src/components/Alerts/AlertDetails/AlertConfigDetails/AlertConfigDetails.tsx Supplies resource-specific event types when rendering existing alert configuration details.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  E[Event emitters] --> M[ResourceEventTypes mapping]
  M --> D[Notification resource descriptor]
  D --> UI[Alert builder options]
  D --> V[AlertUtil save-time validation]
  UI --> S[Subscription request]
  S --> V
  V -->|supported| P[Persist subscription]
  V -->|unsupported| B[400 Bad Request]
Loading

Reviews (8): Last reviewed commit: "Merge branch 'main' into fix/alert-suppo..." | Re-trigger Greptile

@manerow
manerow requested a review from a team as a code owner July 28, 2026 12:46
@manerow manerow added safe to test Add this label to run secure Github workflows on PRs bug Something isn't working backend json schema alerts and notifications labels Jul 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

✅ TypeScript Types Auto-Updated

The generated TypeScript types have been automatically updated based on JSON schema changes in this PR.

@github-actions
github-actions Bot requested a review from a team as a code owner July 28, 2026 12:51
@manerow manerow self-assigned this Jul 28, 2026
@manerow
manerow force-pushed the fix/alert-supported-event-types branch from fbccf37 to 4f1338f Compare July 28, 2026 12:52
@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 65%
66% (77560/117513) 49.95% (46800/93685) 51.15% (14074/27511)

@manerow
manerow force-pushed the fix/alert-supported-event-types branch from f4e0c77 to 0ae5868 Compare July 28, 2026 13:21
@manerow manerow added the UI UI specific issues label Jul 28, 2026
@manerow manerow changed the title fix(alerts): declare the event types each resource can deliver and reject the rest fix(alerts): declare the event types each resource can deliver, reject and stop offering the rest Jul 28, 2026
@manerow
manerow force-pushed the fix/alert-supported-event-types branch from 0ae5868 to 35188bb Compare July 28, 2026 14:04
@gitar-bot

gitar-bot Bot commented Jul 31, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 2 resolved / 2 findings

Aligns notification alert configuration with the event types each resource can actually emit by adding server-side validation and updating the alert builder options. All findings were addressed and no issues remain.

✅ 2 resolved
Quality: clearFields comment contradicts strict enforcement behavior

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EventSubscriptionRepository.java:87-90
The new comment on clearFields claims values the subscription already stores "stay valid on update even if the resource no longer produces them, so an upgrade cannot block edits." That grandfathering was removed in this PR's final commit ("enforce supported event types strictly instead of grandfathering dead values"): createToEntity/prepare now call validateAndBuildFilteringConditions, which rejects unreachable values on update via the mapper on the PUT path. The comment is stale and directly contradicts the documented release-note behavior (an upgraded alert with a dead value gets a 400 on next save). Remove or correct the comment so it no longer implies grandfathering that the code does not perform.

Edge Case: Event-type validation ignores filter Effect (rejects EXCLUDE too)

📄 openmetadata-service/src/main/java/org/openmetadata/service/events/subscription/AlertUtil.java:336-345 📄 openmetadata-service/src/main/java/org/openmetadata/service/events/subscription/AlertUtil.java:355-361
validateEventTypes inspects every filterByEventType filter regardless of its Effect (INCLUDE vs EXCLUDE). An alert that merely EXCLUDEs an event type the resource can never produce is harmless — it can still fire on the other events — yet it will now be rejected with a 400. The feature's goal is to prevent silent no-op INCLUDE alerts; consider validating values only for INCLUDE-effect filters so a benign exclusion of a dead value does not block saving.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar | Powered by Gitar — free for open source

@sonarqubecloud

Copy link
Copy Markdown

@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

alerts and notifications backend bug Something isn't working json schema safe to test Add this label to run secure Github workflows on PRs UI UI specific issues

Projects

None yet

1 participant